Skip to content

feat(server): transfer Orchestrator v2 threads - #7709

Closed
olafura wants to merge 3 commits into
pingdotgg:mainfrom
olafura:olafura/thread-transfer
Closed

feat(server): transfer Orchestrator v2 threads#7709
olafura wants to merge 3 commits into
pingdotgg:mainfrom
olafura:olafura/thread-transfer

Conversation

@olafura

@olafura olafura commented Aug 20, 2026

Copy link
Copy Markdown

Stacked on #7708 (export/import), which is stacked on #7707 (thread:list); review the last commit only until those land.

Problem

The t3code/codex-turn-mapping branch stores Orchestrator v2 threads differently: events carry application_event_version, a migrated thread keeps its legacy v1 events beside the v2 ones, the thread row lives in orchestration_v2_projection_threads, and a migrated thread may still have a pending v1 transcript import. The export/import from #7708 only understands the v1 shape, so it would export a mixed stream and import it onto a schema that cannot hold it. Moving threads between userdata and dev is exactly the case where one side is on that branch and the other is not.

Fix

Export detects the stream's version, exports only that version's events, reads the thread row from the v2 projection table, and refuses a migrated thread whose transcript import is still pending (its messages would be missing from the archive; opening it once in the source server fixes that). Import writes the version column when the destination has it and refuses a v2 archive on a schema that does not; the v2 server rebuilds projections for threads that have events but no rows, so import stays events-only. thread:list, export, and import report each thread's orchestration version.

The v2 tables do not exist on main yet, so every probe here is table-existence tolerant: on a v1-only database the scripts behave exactly as in #7708, and the tests seed both shapes. If the maintainers would rather hold this until the v2 schema lands on main, #7707 and #7708 stand on their own.

Tests added to apps/server/scripts/thread-transfer.test.ts: v2 export/import of a migrated stream, the pending-transcript refusal, importing a v1 thread into a direct v2 dev state directory, and the v2-onto-v1 downgrade refusal.

Claude Fable 5 via Claude Code


Note

Medium Risk
Maintainer scripts that read and write SQLite event logs, attachments, and optional terminal history. Import can mutate a live userdata DB behind an explicit flag, but it backs up first, refuses a busy server, and rolls back files on failure.

Overview
Adds maintainer CLIs to list, export, and import one T3 thread between state directories (userdata or dev), including Orchestrator v2 streams.

Export writes a JSON archive of that thread’s events (v1 or v2 only—not a mixed migrated stream), image attachments, and optional terminal logs. Import remaps the thread onto a destination project, backs up the DB, writes events only (the server rebuilds projections), and copies files with checksums and rollback. Worktree paths that do not exist on the destination are cleared.

Guards: refuse a pending v1 transcript on v2 export, refuse v2 onto a v1 schema, refuse collisions and the live ~/.t3/userdata unless --dangerous-allow-t3-directory, and require the destination server to be stopped. thread:list reports orchestration version so you can pick ids.

Reviewed by Cursor Bugbot for commit 158b81c. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add CLI tools to export, import, and list Orchestrator v2 threads

  • Introduces three CLI commands — thread:list, thread:export, and thread:import — backed by core logic in thread-transfer.ts that handles both v1 and v2 orchestration event schemas.
  • Export serializes a single thread's events, attachments, and optional terminal logs into a JSON ThreadArchive. Import validates the archive, backs up the destination DB via VACUUM INTO, remaps project IDs, clears worktree paths that don't exist on the target, and inserts events transactionally with file rollback on failure.
  • listThreads reads live projects and threads across v1/v2 projection tables, preferring v2 rows where present, and outputs either human-readable tables or JSON via the --json flag.
  • resolveStateLocation accepts workspace root, base dir, or direct state dir forms, probing for state.sqlite in each.
  • Risk: import refuses to write to the shared ~/.t3/userdata unless --dangerous-allow-t3-directory is set; v2 import requires the destination DB to have application_event_version column support, and pending transcripts are refused on export via ensureLegacyTranscriptHydrated in thread-transfer.ts.
📊 Macroscope summarized 158b81c. 6 files reviewed, 2 issues evaluated, 0 issues filtered, 2 comments posted

🗂️ Filtered Issues

olafura and others added 3 commits August 20, 2026 19:14
Add `vp run thread:list`, which prints the live threads of an existing T3
state database with their workspace roots and titles, or the full records
as JSON. The source may be a workspace containing `.t3`, the T3 base
directory, or a direct state directory, and `--state dev` selects a
main-checkout dev database. The database is opened read-only.

This is the first step of thread transfer; export and import build on the
same state-directory resolution.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add `vp run thread:export` and `vp run thread:import`. Export writes one
thread's orchestration events and image attachments (terminal logs only
with `--include-terminal-logs`, since they may hold credentials) into a
self-contained JSON archive with per-file checksums. Import validates the
archive (every event belongs to the thread, decodes against the
orchestration contract, and carries unique ids and stream versions),
refuses a destination that already holds the thread, backs the destination
database up with VACUUM INTO, remaps the thread onto the target project,
clears worktree paths that do not exist on the destination, and writes
only the events: the destination server replays them above the
projectors' recorded sequence on its next start and rebuilds the read
model itself. Copying projection rows too would make that replay append
onto already-complete rows.

The live ~/.t3/userdata database is refused unless
`--dangerous-allow-t3-directory` is passed, which is how a thread moves
from a dev checkout back into the real install once its server is
stopped. The source and destination accept the same directory forms and
`--state dev` selection as `thread:list`. `ensureDevDbNotInUse` is the
renamed dev-db guard from migrate-dev-db.ts, reused to refuse importing
into a running server's database.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Teach thread transfer about the Orchestrator v2 schema of the
codex-turn-mapping branch. Export detects whether a stream carries
application_event_version 2 events, exports only that version's events
(a migrated thread keeps its legacy v1 events beside the v2 ones), reads
the thread row from orchestration_v2_projection_threads, and refuses a
migrated thread whose transcript import is still pending, since its
messages would be missing from the archive. Import writes the version
column when the destination has it and refuses a v2 archive on a schema
that does not; the v2 server rebuilds projections for threads that have
events but no rows, so import stays events-only. List, export, and import
report each thread's orchestration version. Every probe is
table-existence tolerant, so the scripts keep working on a v1-only
database.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9a6e2a9f-9178-4afb-92ed-1193165ca9f9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). labels Aug 20, 2026
event.correlationId,
event.actorKind,
event.payloadJson,
event.metadataJson,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High scripts/thread-transfer.ts:856

V2 imports commit syntactically invalid metadataJson (and potentially payloadJson) because ensureEventsDecode is skipped for orchestration version 2 and insertEvents writes both strings verbatim. Later event-store reads fail to decode metadata_json, preventing the destination from rebuilding or reading the thread. JSON-decode both fields before insertion for v2 archives, even without validating against a v2 event contract.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/scripts/thread-transfer.ts around line 856:

V2 imports commit syntactically invalid `metadataJson` (and potentially `payloadJson`) because `ensureEventsDecode` is skipped for orchestration version 2 and `insertEvents` writes both strings verbatim. Later event-store reads fail to decode `metadata_json`, preventing the destination from rebuilding or reading the thread. JSON-decode both fields before insertion for v2 archives, even without validating against a v2 event contract.

}

const pendingFiles = [
...(yield* stageArchiveFiles(location, ATTACHMENTS, archive.attachments, archive.thread.id)),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium scripts/thread-transfer.ts:920

Duplicate fileName entries are accepted, so a later staged file silently overwrites an earlier one and the import can succeed with contents different from the archive. Reject duplicate destination paths while staging each file family.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/scripts/thread-transfer.ts around line 920:

Duplicate `fileName` entries are accepted, so a later staged file silently overwrites an earlier one and the import can succeed with contents different from the archive. Reject duplicate destination paths while staging each file family.

@macroscopeapp macroscopeapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the new apps/server/scripts/thread-transfer.ts and its CLI wrappers against the Effect service conventions. Imports, namespace usage, dependency acquisition (no ManagedRuntime/runPromise, no service-instance injection), and file layout look consistent with the existing scripts. Two error-modeling issues below.

Posted via Macroscope — Effect Service Conventions

Comment on lines +882 to +884
yield* ensureDevDbNotInUse(location.databasePath).pipe(
Effect.mapError((cause) => transferError("import thread", cause.message, cause)),
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cause.message is copied into detail, and message is derived from detail, so the wrapper's message comes from the underlying error rather than its own structural attributes. ensureDevDbNotInUse already fails with structured, caller-visible errors (MigrateDevDbServerRunningError, MigrateDevDbDestinationBusyError), so letting them pass through preserves that structure; if a wrapper is required here, build detail from the destination path and keep the original error as cause.

Suggested change
yield* ensureDevDbNotInUse(location.databasePath).pipe(
Effect.mapError((cause) => transferError("import thread", cause.message, cause)),
);
yield* ensureDevDbNotInUse(location.databasePath);

Posted via Macroscope — Effect Service Conventions

Comment on lines +93 to +104
export class ThreadTransferError extends Schema.TaggedErrorClass<ThreadTransferError>()(
"ThreadTransferError",
{
operation: Schema.String,
detail: Schema.String,
cause: Schema.optional(Schema.Defect()),
},
) {
override get message(): string {
return `${this.operation}: ${this.detail}`;
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ThreadTransferError stores the whole failure as a free-form detail string and derives message from it, so the unstructured message is the only real data; operation is also an unconstrained string. Consider following the neighbouring migrate-dev-db.ts precedent and modelling the genuinely distinct failures (missing state database, thread already exists, checksum mismatch, contract mismatch, v2 downgrade) as their own tagged errors with structural attributes (databasePath, threadId, fileName), or at minimum narrow operation to Schema.Literals([...]) and move the variable context into dedicated fields. That also removes the need for the transferError pass-through factory below (line 201), which only forwards constructor arguments — conventions ask for the error to be constructed at the failure boundary so its attributes and cause stay visible.

Posted via Macroscope — Effect Service Conventions

@macroscopeapp

macroscopeapp Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR introduces a substantial new thread transfer feature (~2200 lines) with database operations, v1/v2 orchestration version handling, and file migration logic. New features of this scope warrant human review. Additionally, there is a High severity finding about v2 imports potentially writing invalid JSON.

Not approved because:

  • 2 blocking correctness issues found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

@t3dotgg

t3dotgg commented Aug 23, 2026

Copy link
Copy Markdown
Member

Note

🤖 GPT-5.6 Sol responding on behalf of Theo

Closing this PR after an automated pass over open pull requests. Rewrites core orchestration or introduces a new thread, goal, or message-lifecycle model.

@t3dotgg t3dotgg closed this Aug 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL 1,000+ changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants